feat: adjusted grade field validation #382

dyudyunov · 2024-02-07T13:47:34Z

Description

Added simple validation for the "Adjusted grade" field in the "Edit Grades" popup:

allow only a numeric value
define the minimum value (0)
checking the correctness of the entered possibleGrade value

Notes

This was already merged in nutmeg and maple, but the PR to the master branch wasn't open at that time.

Steps to reproduce

Go to Instructor -> Student admin -> View Gradebook
Click on any user
In the Adjusted grade field fill in symbols, letters, a large number

Behaviour before the fix

Entering the large number has no errors and it's possible to save the incorrect value:

Entering text/special symbols results in 500 errors for the request:

openedx-webhooks · 2024-02-07T13:47:39Z

Thanks for the pull request, @dyudyunov!

What's next?

Please work through the following steps to get your changes ready for engineering review:

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

🔘 Let us know that your PR is ready for review:

Who will review my changes?

This repository is currently maintained by @farhaanbukhsh. Tag them in a comment and let them know that your changes are ready for review.

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

codecov · 2024-02-07T13:51:52Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.02%. Comparing base (ba9bd46) to head (5aba8f2).
Report is 32 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #382      +/-   ##
==========================================
+ Coverage   94.98%   95.02%   +0.03%     
==========================================
  Files         140      140              
  Lines        1357     1366       +9     
  Branches      264      269       +5     
==========================================
+ Hits         1289     1298       +9     
  Misses         60       60              
  Partials        8        8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

farhaanbukhsh · 2024-03-04T11:06:24Z

@mphilbrick211 I can take the review for this.

arbrandes · 2024-03-04T17:11:47Z

@dyudyunov, do you mind adding testing instructions and screenshots to the PR description?

mphilbrick211 · 2024-03-04T21:53:22Z

Thank you, @farhaanbukhsh!

dyudyunov · 2024-03-05T08:10:58Z

hi @arbrandes

I updated the description, please check

arbrandes · 2024-03-05T14:42:24Z

In trying to reproduce this I found another bug, but yeah, this is definitely an issue.

arbrandes · 2024-03-05T14:52:28Z

Here's what I'm thinking:

The proposed changes don't fix the symbols problem. You can still enter garbage, and you'll still get a 500 error.
The validation is doing more than just validation. I believe we should be blocking submission and showing an error message, instead of trying to guess what the user was trying to do.

Is this something you have time to tackle here?

arbrandes

See comment above.

dyudyunov · 2024-04-01T06:32:19Z

I lost this issue somehow. Added it to my TODO list now. I'll give updates asap

dyudyunov · 2024-04-01T12:53:41Z

@arbrandes could you clarify your proposals?

The proposed changes don't fix the symbols problem. You can still enter garbage, and you'll still get a 500 error

With current changes, the "Adjusted grade" field only allows input of numbers in a dedicated range.
How can I reproduce the issue you're describing?

mphilbrick211 · 2024-05-02T00:33:01Z

@arbrandes could you clarify your proposals?

The proposed changes don't fix the symbols problem. You can still enter garbage, and you'll still get a 500 error

With current changes, the "Adjusted grade" field only allows input of numbers in a dedicated range. How can I reproduce the issue you're describing?

Bumping this for you, @arbrandes!

arbrandes · 2024-05-15T17:24:15Z

src/components/GradesView/EditModal/OverrideTable/AdjustedGradeInput/hooks.js

+        adjustedGradeValue = 0;
+        break;
+      case possibleGrade && target.value > possibleGrade:
+        adjustedGradeValue = possibleGrade;


My point was that instead of artificially modifying the number the user was trying to enter, we should show an error message instead. This is so they can understand what the problem was.

Got it

The proposal makes sense, but I can't say when I'll have time to implement it (too loaded now with other stuff)

arbrandes · 2024-05-15T17:25:30Z

src/components/GradesView/EditModal/OverrideTable/AdjustedGradeInput/index.jsx

  } = useAdjustedGradeInputData();
  return (
    <span>
      <Form.Control
-        type="text"
+        type="number"


When I tested this, I was still able to enter non-numeric characters. It could be that this is a Paragon bug.

It's strange, I'm sure I tested that 🤔
I was able to use numbers, ., and , only

Maybe it's because of the different Paragon versions. If I remember correctly - I tested it on Quince

arbrandes · 2024-05-15T17:29:17Z

I believe #366 is more in line with what I'm thinking.

mphilbrick211 · 2024-07-30T20:36:55Z

Hi @dyudyunov and @arbrandes - is this still in progress?

farhaanbukhsh · 2024-10-28T10:55:48Z

@dyudyunov @mphilbrick211 @arbrandes can we close this PR in favour of #366?

dyudyunov · 2024-11-04T09:06:45Z

@dyudyunov @mphilbrick211 @arbrandes can we close this PR in favour of #366?

no objections from my side)

Inferato and others added 4 commits October 23, 2023 16:37

feat: adjusted grade field validation

62656ce

feat: adjusted grade field validation

8a3987e

test: update snapshot

cbc4939

test: increase coverage

5aba8f2

dyudyunov requested a review from a team as a code owner February 7, 2024 13:47

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Feb 7, 2024

This was referenced Feb 7, 2024

feat: adjusted grade field validation #381

Closed

Lytvynenko/grade field validation master #364

Closed

mphilbrick211 added the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Feb 21, 2024

arbrandes self-requested a review March 4, 2024 17:09

arbrandes added the bug Report of or fix for something that isn't working as intended label Mar 5, 2024

arbrandes requested changes Mar 5, 2024

View reviewed changes

mphilbrick211 removed the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Mar 5, 2024

arbrandes requested changes May 15, 2024

View reviewed changes

farhaanbukhsh closed this Nov 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: adjusted grade field validation #382

feat: adjusted grade field validation #382

dyudyunov commented Feb 7, 2024 •

edited

Loading

openedx-webhooks commented Feb 7, 2024 •

edited

Loading

codecov bot commented Feb 7, 2024 •

edited

Loading

farhaanbukhsh commented Mar 4, 2024

arbrandes commented Mar 4, 2024

mphilbrick211 commented Mar 4, 2024

dyudyunov commented Mar 5, 2024

arbrandes commented Mar 5, 2024

arbrandes commented Mar 5, 2024

arbrandes left a comment

dyudyunov commented Apr 1, 2024

dyudyunov commented Apr 1, 2024 •

edited

Loading

mphilbrick211 commented May 2, 2024

arbrandes May 15, 2024

dyudyunov May 17, 2024

arbrandes May 15, 2024

dyudyunov May 17, 2024

dyudyunov May 17, 2024

arbrandes commented May 15, 2024

mphilbrick211 commented Jul 30, 2024

farhaanbukhsh commented Oct 28, 2024

dyudyunov commented Nov 4, 2024

feat: adjusted grade field validation #382

feat: adjusted grade field validation #382

Conversation

dyudyunov commented Feb 7, 2024 • edited Loading

Description

Notes

Steps to reproduce

Behaviour before the fix

openedx-webhooks commented Feb 7, 2024 • edited Loading

What's next?

🔘 Get product approval

🔘 Provide context

🔘 Get a green build

🔘 Let us know that your PR is ready for review:

Who will review my changes?

Where can I find more information?

When can I expect my changes to be merged?

codecov bot commented Feb 7, 2024 • edited Loading

Codecov Report

farhaanbukhsh commented Mar 4, 2024

arbrandes commented Mar 4, 2024

mphilbrick211 commented Mar 4, 2024

dyudyunov commented Mar 5, 2024

arbrandes commented Mar 5, 2024

arbrandes commented Mar 5, 2024

arbrandes left a comment

Choose a reason for hiding this comment

dyudyunov commented Apr 1, 2024

dyudyunov commented Apr 1, 2024 • edited Loading

mphilbrick211 commented May 2, 2024

arbrandes May 15, 2024

Choose a reason for hiding this comment

dyudyunov May 17, 2024

Choose a reason for hiding this comment

arbrandes May 15, 2024

Choose a reason for hiding this comment

dyudyunov May 17, 2024

Choose a reason for hiding this comment

dyudyunov May 17, 2024

Choose a reason for hiding this comment

arbrandes commented May 15, 2024

mphilbrick211 commented Jul 30, 2024

farhaanbukhsh commented Oct 28, 2024

dyudyunov commented Nov 4, 2024

dyudyunov commented Feb 7, 2024 •

edited

Loading

openedx-webhooks commented Feb 7, 2024 •

edited

Loading

codecov bot commented Feb 7, 2024 •

edited

Loading

dyudyunov commented Apr 1, 2024 •

edited

Loading